Skip to content

Bug#121063 Members of one group assign different GNOs to the same transaction - #736

Open
matias-sanchez wants to merge 3 commits into
mysql:trunkfrom
matias-sanchez:bug121063-stale-synode-reservation
Open

Bug#121063 Members of one group assign different GNOs to the same transaction#736
matias-sanchez wants to merge 3 commits into
mysql:trunkfrom
matias-sanchez:bug121063-stale-synode-reservation

Conversation

@matias-sanchez

Copy link
Copy Markdown

Bug#121063: https://bugs.mysql.com/bug.php?id=121063

What happens

Two members of the same group assign different GNOs to the same transaction, in multi-primary mode, when a member leaves and rejoins during a rolling restart under write load. The group splits into internally consistent sets that disagree on the GTID of the same transaction. It can surface on the group_replication_applier channel as Error_code 1032 or 1062, or stay silent with every member ONLINE and the disagreement present only in the binary logs.

Reproduced on 8.4.8, 8.4.11 and 9.7.2. A self contained reproducer is attached to the bug.

Where it starts

A reserved synode is only ours while we still hold the node index it was reserved under.

local_synode_allocator stamps the member's current index into the synode, synode.node = my_nodeno. Still inside reserve_synode_number, the task yields in the while (too_far(*msgno)) loop at TIMED_TASK_WAIT. During that yield, site_install_action reassigns site->nodeno. The reservation still carries the old index, so proposer_task brands and proposes into a slot that now belongs to another node.

The header comment of xcom_base.cc states the rule at line 107: only node N may propose a value for synode {X N}. With two proposers on the same slot at cnt=0, acceptor.promise is never raised, since it is assigned in exactly one place, inside handle_simple_prepare, which is the phase 1 decision. Both proposals are accepted, both are learned, and handle_learn keeps whichever LEARN arrived first because of the /* Avoid re-learn */ guard. Members that heard different values first deliver different payloads.

A bpftrace trace of one such slot followed from ACCEPT through LEARN to DELIVER is in the bug, posted 28 Aug 2026.

The change

Before proposing, verify the reservation still carries the member's own node index. If it does not, drop it through retry_new and take a new one.

This is the same check incr_msgno (xcom_base.cc:668) already makes whenever it advances, with the comment In case site and node number has changed. The client transaction is not lost, it goes out in a slot that does belong to the member.

Testing

In the lab the anomaly goes to zero: 0 stale proposals out of 15,523,048, against 993 predicted from the unpatched rate, and 0 divergences out of 5,667,504 transactions, against 17.1 expected. No run carrying the patch has reproduced the divergence, on two hosts.

Performance: 12 runs per arm across two hosts, load only, no restarts, no emulated latency, comparing the patched plugin against the same source built without the patch and against the stock image. Ratio of median successful operations per 10 s window, bootstrap over runs: patched over unpatched [0.9998, 1.0004], patched over stock [0.9994, 1.0003]. A positive control with a known throttle was detected at [0.9944, 0.9953], so the method resolves differences of about 0.5%. The workload is rate limited, so this measures behaviour under a production-like load and not peak capacity.

I do not have an MTR test for this. The reproduction needs seven members, rolling restarts and sustained write load, which does not fit the standard framework. The reproducer attached to the bug builds that environment and decides the verdict by decoding every member's binary log and comparing all 21 pairs.

@matias-sanchez
matias-sanchez requested a review from a team August 28, 2026 20:57
@oracle-contributor-agreement oracle-contributor-agreement Bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Aug 28, 2026
@github-actions github-actions Bot added Replication Changes touching replication or binlog code Pluggable Changes touching plugins or components Review Requested Review requested from code owners labels Aug 28, 2026
@gopshank
gopshank requested review from nacarvalho and removed request for gopshank and seemasundara August 29, 2026 01:21
@nacarvalho
nacarvalho requested review from jujose-1 and tiagoportelajorge and removed request for nacarvalho August 31, 2026 09:09

@jujose-1 jujose-1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Matias,
Thank you for the contribution and for working on this fix. I’ve left a few review comments for your consideration. Please take a look when you have a chance.

Regards,
Justin Jose

@matias-sanchez
matias-sanchez force-pushed the bug121063-stale-synode-reservation branch from 288df77 to 267fed8 Compare September 2, 2026 23:41
@matias-sanchez

Copy link
Copy Markdown
Author

Hi @jujose-1, thanks for taking the time to review this and for the comments. Added the requested changes.

@jujose-1 jujose-1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @matias-sanchez ,

Thank you for updating the patch and addressing the earlier feedback. The production change looks good to me. I’ve left one additional comment regarding the unit-test setup.

I’ll also wait for @tiagoportelajorge 's review and any additional feedback he may have.

Thank you again for working on this.

Regards,
Justin Jose

Comment thread unittest/gunit/libmysqlgcs/xcom/gcs_xcom_stale_reservation-t.cc Outdated
@kamil-holubicki

Copy link
Copy Markdown

My proposal is to add two debug asserts:

  1. Proposer side (offending node) in prepare_push_2p() after the ballot setup: assert(p->proposer.bal.node == p->synode.node). The node can propose only for its synode. Detect the case when the current node index mismatches the index at the synode reservation time.

  2. Acceptor side in handle_simple_accept(): assert(m->proposal.cnt != 0 || m->proposal.node == m->synode.node);. In fast path (cnt==0) the node can propose only for its synode.

@github-actions github-actions Bot added the MTR Failed MTR suite failed label Sep 3, 2026
@github-actions github-actions Bot removed the MTR Failed MTR suite failed label Sep 4, 2026
@matias-sanchez

matias-sanchez commented Sep 4, 2026

Copy link
Copy Markdown
Author

Thanks @kamil-holubicki . However, I'm not sure on how to safely add these asserts, because the issue covered is on multi primary and I'm not sure how to add that assert so that it does not fire when paxos_single_leader is ON, as in that mode the synode carries the leader's node index, not the proposer's. @jujose-1 would adding site->max_active_leaders != active_leaders_all be the proper way to validate this? Also, do you recommend adding this on this PR or would you prefer a follow up one?

@jujose-1

jujose-1 commented Sep 4, 2026

Copy link
Copy Markdown
Member

Thanks, @matias-sanchez , for updating the unit test. The revised test now uses valid memberships and verifies the same reservation across the view change. This looks good to me.

Thanks also, @kamil-holubicki, for the suggestions.

@jujose-1 would adding site->max_active_leaders != active_leaders_all be the proper way to validate this? Also, do you recommend adding this on this PR or would you prefer a follow up one?

My understanding is that the proposed assertions would cover the scenario reported in this bug, but they may not hold for remotely allocated synodes, where the synode can legitimately carry the allocating leader’s node index rather than the proposer’s.

Given that the reported issue involves a locally allocated reservation, and the current fix scopes the ownership validation to that path, I suggest keeping this PR focused on that change. The acceptor side does not currently have the allocation provenance needed to apply the same invariant safely, so I would prefer not to add the proposed assertions here.

kamil-holubicki pushed a commit to kamil-holubicki/percona-server that referenced this pull request Sep 4, 2026
…t GNOs to the same transaction

https://perconadev.atlassian.net/browse/PS-11530

Upstream bug:
Bug#121063: https://bugs.mysql.com/bug.php?id=121063

Problem:
Two members of the same group assign different GNOs to the same
transaction, in multi-primary mode, when a member leaves and rejoins
during a rolling restart under write load. The group splits into
internally consistent sets that disagree on the GTID of the same
transaction. It can surface on the `group_replication_applier` channel
as Error_code 1032 or 1062, or stay silent with every member ONLINE and
the disagreement present only in the binary logs.

Cause:
A reserved synode is only ours while we still hold the node index it was
reserved under.

`local_synode_allocator` stamps the member's current index into the
synode, `synode.node = my_nodeno`. Still inside `reserve_synode_number`,
the task yields in the `while (too_far(*msgno))` loop at
`TIMED_TASK_WAIT`. During that yield, `site_install_action` reassigns
`site->nodeno`. The reservation still carries the old index, so
`proposer_task` brands and proposes into a slot that now belongs to
another node.

The header comment of `xcom_base.cc` states the rule: only node N may
propose a value for synode {X N}. With two proposers on the same slot
at `cnt=0`, `acceptor.promise` is never raised, since it is assigned in
exactly one place, inside `handle_simple_prepare`, which is the phase 1
decision. Both proposals are accepted, both are learned, and
`handle_learn` keeps whichever LEARN arrived first because of the
`/* Avoid re-learn */` guard. Members that heard different values first
deliver different payloads.

Solution:
Before proposing, verify the reservation still carries the member's own
node index. If it does not, drop it through `retry_new` and take
a new one.

This is the same check `incr_msgno` already makes whenever it advances,
with the comment `In case site and node number has changed`.
The client transaction is not lost, it goes out in a slot that does
belong to the member.

Ported from upstream MySQL PR
mysql/mysql-server#736
by Matias Sanchez.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

OCA Verified All contributors have signed the Oracle Contributor Agreement. Pluggable Changes touching plugins or components Replication Changes touching replication or binlog code Review Requested Review requested from code owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants